Skip to content

#906 Fix video embedding.#1415

Open
dandry wants to merge 2 commits intoCapSoftware:mainfrom
dandry:fix/embed
Open

#906 Fix video embedding.#1415
dandry wants to merge 2 commits intoCapSoftware:mainfrom
dandry:fix/embed

Conversation

@dandry
Copy link

@dandry dandry commented Nov 27, 2025 •
edited by coderabbitai bot
Loading

I believe this change fixes the issue mentioned in #906.

I was not able to verify this, though, as I could not spin up the images I built via GH actions in my forked repository - received an error when I tried to access Cap's dashboard.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed access control to allow embedded content paths to bypass login redirect in supported environments.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 27, 2025 •
edited
Loading

Walkthrough

Modified apps/web/middleware.ts to add "/embed/" as a path exception in the access control redirect logic. This permits requests to /embed/... to bypass the login redirect in non-CAP environments, where they previously would have redirected to /login.

Changes

Cohort / File(s) Change Summary
Middleware path exceptions
apps/web/middleware.ts		 Added "/embed/" to the allowed path exceptions in access control redirect logic, allowing embedded content requests to bypass login authentication in non-CAP environments.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single-file change with minimal scope
  • Simple path exception addition to existing logic
  • Low risk of unintended side effects; straightforward exception list modification

Possibly related PRs

  • Self hosted fixes #1332: Also modifies apps/web/middleware.ts to update access-control allowed-path exceptions, suggesting a related pattern of path-based access control refinement.

Poem

🐰 A rabbit hops through /embed/ so free,
No login gates for thee and me!
The middleware bends, exceptions grow,
Access flows where admins let it go. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically relates to the main change: adding /embed/ path exception to middleware, which directly fixes video embedding functionality. The reference to issue #906 provides clear context.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a292974 and 66d8979.

📒 Files selected for processing (1)
  • apps/web/middleware.ts (1 hunks)
🔇 Additional comments (1)
apps/web/middleware.ts (1)

40-59: /embed/ allowlist entry matches PR intent; consider bare /embed edge case

Adding path.startsWith("/embed/") to the non-CAP allowlist correctly prevents /embed/... from being forced to /login in production self-hosted setups and is consistent with the existing /s/ behavior.

One minor thing to double-check: if any route uses the exact path /embed (without a trailing slash or segment), it will still be redirected to /login because startsWith("/embed/") won’t match. If such a route exists, you might want to broaden the check (e.g., also permit path === "/embed").

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dandry
Copy link
Author

dandry commented Dec 29, 2025

Is there anything I can do to help push this forward?

@dandry dandry mentioned this pull request Dec 29, 2025
Open
@andrewmitchell-automations
Copy link

andrewmitchell-automations commented Jan 26, 2026

Adding a note that this is a huge blocker for me right now. I'm guessing I speak for @dandry when I say whatever we can do to move this small fix forward, we're happy to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dandry @andrewmitchell-automations